-
-
Notifications
You must be signed in to change notification settings - Fork 598
Add Starred API, revised Watchers API and Improved Docs #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
valtlfelipe
commented
Sep 17, 2014
- Added Starred API for get starred repos, star an repo and unstar an repo for authenticated users.
- Revised Watchers API using a new endpoint as shown in the GitHub API.
- Improved Documentation | Added incomplete Activity documentation for Starred and Watchers API.
return $this->get('user/starred', array( | ||
'page' => $page | ||
)); | ||
return new Starred($this->client); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the return value of an existing method is a BC break
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, i will fix this
Fixing the watchers API is already covered in #183 |
I just commited restoring back the method and creating a new one so it does not break the usage. |
|
||
|
||
/** | ||
* @Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should write it as @deprecated Use watchers() instead
I just commited removing the unused class (I forgot to remove it from git) and updated the docblock's. |
|
||
|
||
/** | ||
* @Deprecated Use watchers() instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you use same lowercase as other docblocks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, sorry about this. I saw you already fixed it! Thanks ;)
Add Starred API, revised Watchers API and Improved Docs
good job |
Thank you! |